-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Clean up output capture on .NET Core #78205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This cleans up how we handle output capture on .NET Core. This moves from a lot of mutable `static` static, that would break if tests ran in parallel, to instance state that uses locks to guard againtst concurrent runs.
@dotnet/roslyn-compiler PTAL |
public interface IRuntimeEnvironment : IDisposable | ||
{ | ||
(int ExitCode, string Output, string ErrorOutput) Execute(string[] args, int? maxOutputLength); | ||
(int ExitCode, string Output, string ErrorOutput) Execute(string[] args); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously the maxOutputLength
parameter was used to guard againts infinite loops, seemed useful but it's removed now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was only done on .NET Core though, not .NET Framework. I considered putting the same code path in .NET Framework but that guard hasn't gotten tripped anytime that I remember. Removing the guard means we will still get an exception in an infinite loop, just it will be an OOM exception. So decided to simplify here and remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fwiw, I've seen that guard hit locally during development, I think.
This cleans up how we handle output capture on .NET Core. This moves from a lot of mutable
static
static, that would break if tests ran in parallel, to instance state that uses locks to guard againtst concurrent runs.